Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CIDRPool 1/x] Add CIDRPool CRD definition and validation logic #41

Merged
merged 1 commit into from
Jun 11, 2024
Merged

[CIDRPool 1/x] Add CIDRPool CRD definition and validation logic #41

merged 1 commit into from
Jun 11, 2024

Conversation

ykulazhenkov
Copy link
Collaborator

No description provided.

@coveralls
Copy link

Coverage Status

coverage: 70.619% (-1.5%) from 72.083%
when pulling 87dc4ca on ykulazhenkov:pr-cidr-pool-api
into 18fc6a0 on Mellanox:main.

@ykulazhenkov ykulazhenkov mentioned this pull request Jun 10, 2024
Merged
@ykulazhenkov ykulazhenkov changed the title Add CIDRPool CRD definition and validation logic [CIDRPool 1/x] Add CIDRPool CRD definition and validation logic Jun 10, 2024
vasrem
vasrem approved these changes Jun 10, 2024
View reviewed changes
Copy link
Collaborator

@vasrem vasrem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Mostly nitpicks, some ideas as well.

examples/cidrpool-1.yaml Outdated Show resolved Hide resolved
examples/cidrpool-1.yaml Show resolved Hide resolved
api/v1alpha1/cidrpool_test.go Outdated Show resolved Hide resolved
api/v1alpha1/cidrpool_test.go Outdated Show resolved Hide resolved
api/v1alpha1/cidrpool_type.go Outdated Show resolved Hide resolved
api/v1alpha1/cidrpool_validate.go Outdated Show resolved Hide resolved
api/v1alpha1/helpers_test.go Show resolved Hide resolved
api/v1alpha1/helpers.go Outdated Show resolved Hide resolved
api/v1alpha1/cidrpool_type.go Outdated Show resolved Hide resolved
api/v1alpha1/cidrpool_test.go Outdated Show resolved Hide resolved
@coveralls
Copy link

Coverage Status

coverage: 70.525% (-1.6%) from 72.083%
when pulling 030420c on ykulazhenkov:pr-cidr-pool-api
into 18fc6a0 on Mellanox:main.

@coveralls
Copy link

Coverage Status

coverage: 70.538% (-1.5%) from 72.083%
when pulling f975526 on ykulazhenkov:pr-cidr-pool-api
into 18fc6a0 on Mellanox:main.

@coveralls
Copy link

Coverage Status

coverage: 70.538% (-1.5%) from 72.083%
when pulling a1f10b2 on ykulazhenkov:pr-cidr-pool-api
into 18fc6a0 on Mellanox:main.

ykulazhenkov
ykulazhenkov commented Jun 10, 2024
View reviewed changes
Copy link
Collaborator Author

@ykulazhenkov ykulazhenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vasrem Thanks for the review. I think I addressed all your comments. Please, have another look on the PR when you have time.

@ykulazhenkov ykulazhenkov requested a review from vasrem June 10, 2024 13:44
@coveralls
Copy link

Coverage Status

coverage: 70.538% (-1.5%) from 72.083%
when pulling cc6ed19 on ykulazhenkov:pr-cidr-pool-api
into 18fc6a0 on Mellanox:main.

@coveralls
Copy link

Coverage Status

coverage: 70.538% (-1.5%) from 72.083%
when pulling a67e27b on ykulazhenkov:pr-cidr-pool-api
into 18fc6a0 on Mellanox:main.

rollandf
rollandf approved these changes Jun 11, 2024
View reviewed changes
api/v1alpha1/cidrpool_type.go Outdated Show resolved Hide resolved
api/v1alpha1/cidrpool_type.go Outdated Show resolved Hide resolved
api/v1alpha1/cidrpool_type.go Outdated Show resolved Hide resolved
@ykulazhenkov
Add CIDRPool CRD
bed2490 
Signed-off-by: Yury Kulazhenkov <ykulazhenkov@nvidia.com>
@ykulazhenkov ykulazhenkov merged commit 3ddd30f into Mellanox:main Jun 11, 2024
9 checks passed
@ykulazhenkov ykulazhenkov deleted the pr-cidr-pool-api branch June 11, 2024 09:34
@coveralls
Copy link

Coverage Status

coverage: 70.538% (-1.5%) from 72.083%
when pulling bed2490 on ykulazhenkov:pr-cidr-pool-api
into 18fc6a0 on Mellanox:main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vasrem vasrem vasrem approved these changes

@rollandf rollandf rollandf approved these changes

@adrianchiris adrianchiris Awaiting requested review from adrianchiris

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ykulazhenkov @coveralls @vasrem @rollandf